Skip to content

fix(actions): treat setup-node's NODE_AUTH_TOKEN placeholder as unset#27

Merged
jan-kubica merged 2 commits into
mainfrom
fix/setup-node-placeholder
May 13, 2026
Merged

fix(actions): treat setup-node's NODE_AUTH_TOKEN placeholder as unset#27
jan-kubica merged 2 commits into
mainfrom
fix/setup-node-placeholder

Conversation

@jan-kubica
Copy link
Copy Markdown
Contributor

Surfaced when dispatching `stella/stdnum#95`'s release against `v0.0.1` — the composite refused to run with:

```
NPM_TOKEN/NODE_AUTH_TOKEN must not be set when using the hardened publish action
```

Root cause

`actions/setup-node@v6` with `registry-url:` configured unconditionally exports:

```
NODE_AUTH_TOKEN=XXXXX-XXXXX-XXXXX-XXXXX
```

as a literal placeholder string, so its `.npmrc` template `_authToken=${NODE_AUTH_TOKEN}` expands to a non-empty but useless value. Every publishing repo in the org uses this setup-node + registry-url pattern, so the previous unconditional refuse-on-NODE_AUTH_TOKEN guard breaks all of them.

Fix

`unset NODE_AUTH_TOKEN` if and only if it matches the literal placeholder. Real legacy tokens (anything else non-empty) still trip the refuse branch.

Test plan

  • shellcheck clean
  • post-merge: bump SHA pin in stella/stdnum's release.yml, re-dispatch against `v0.0.1`, confirm idempotency early-return succeeds

`actions/setup-node@v6` with `registry-url:` configured
unconditionally exports `NODE_AUTH_TOKEN=XXXXX-XXXXX-XXXXX-XXXXX` as
a literal placeholder so its `.npmrc` template
`_authToken=${NODE_AUTH_TOKEN}` expands to a non-empty (but useless)
string. The previous guard treated that placeholder as a real token
and refused to run, breaking every standard setup-node + publish
flow — the placeholder is what the stdnum dispatch hit on
attempt #1.

Unset NODE_AUTH_TOKEN if and only if it matches the placeholder.
Real legacy tokens still trip the refuse-and-exit branch.

Surfaced by the stdnum#95 release dispatch against v0.0.1.
@jan-kubica jan-kubica requested a review from nnad3N as a code owner May 13, 2026 14:47
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the publish.sh script to handle a specific placeholder value exported by actions/setup-node@v6. It now unsets the NODE_AUTH_TOKEN if it matches the placeholder, ensuring that the security guard does not block standard publishing flows. The reviewer suggested declaring the placeholder variable as readonly to improve script robustness and prevent accidental modification.

Comment thread .github/actions/npm-publish-hardened/publish.sh Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a6a109e56

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/actions/npm-publish-hardened/publish.sh Outdated
Codex caught a real subtlety on the previous fix attempt. npm config
does env-var expansion when reading .npmrc, and an unset NODE_AUTH_TOKEN
can let npm pass the literal `${NODE_AUTH_TOKEN}` syntax through to
the Authorization header instead of treating it as absent — which
defeats OIDC and risks the placeholder-token bearer auth that this
action exists to prevent.

Set NODE_AUTH_TOKEN to an empty string instead. `.npmrc`'s
`_authToken=${NODE_AUTH_TOKEN}` expands cleanly to `_authToken=`
(no auth), and npm's OIDC trusted-publishing path takes over via the
ACTIONS_ID_TOKEN_REQUEST_* env vars.

Also adopts gemini's suggestion to declare the placeholder constant
as `readonly`.

Addresses bot reviews on PR #27.
@jan-kubica jan-kubica merged commit 8639294 into main May 13, 2026
1 check passed
@jan-kubica jan-kubica deleted the fix/setup-node-placeholder branch May 13, 2026 14:58
@github-actions github-actions Bot locked and limited conversation to collaborators May 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant